Skip to content

Conversation

phlfm
Copy link
Collaborator

@phlfm phlfm commented Sep 9, 2025

⚠️ Warning - these changes have not been tested on the corresponding hardware / setup / boards!

PR Description

This PR addresses timing violations observed across multiple projects following the upgrade to Vivado 2025.1, without requiring any HDL modifications.

After the upgrade, most designs exhibited hold violations, likely due to Vivado's aggressive optimization for setup timing. This behavior appears to shorten certain paths excessively, inadvertently introducing hold issues. While invoking phys_opt_design post-route helped resolve some violations automatically, it was not consistently effective, particularly in congested regions where rerouting options were limited.

To mitigate this, the Congestion_SpreadLogic_high strategy was adopted (alternatively _medium or _low depending on design needs). This approach encourages a more distributed placement of logic, which not only alleviates congestion but can also accelerate implementation due to the relatively low resource utilization. Although this spreading may introduce setup timing challenges, a subsequent phys_opt_design pass (or two) has proven highly effective at resolving them.

By spreading the logic and reducing congestion, Vivado gains greater flexibility during post-route optimization, improving its ability to correct HOLD violations without manual intervention.

Change log

  • implemented scripts/auto_timing_fix.tcl
  • improvements to projects/scripts/adi_project_xilinx.tcl
    • added ADI_MAX_THREADS so the trade-off between speed and RAM usage can be adjusted
    • added post place and route script flag
    • added documentation for other flags
  • fixed timing issues in fmcomms2 with 2025.1 toolchain without hdl changes
    • project presented hold timing violations on vivado 2025.1
    • "phys_opt_design -hold_fix" resolved it for most variations but not for all of them
    • changed implementation strategy to spread placement on board to make hold timing easier to resolve
    • now all projects finish successfully after the auto fix script runs
    • no HDL changes were made
  • fixed timing for ad9081 on vcu118 for specific make parameters
    • presented setup timing violations
    • use of "spread high" strategy solved issues without need of ATF or HDL changes

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)
  • Documentation

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

@CLAassistant
Copy link

CLAassistant commented Sep 9, 2025

CLA assistant check
All committers have signed the CLA.

@IuliaCMoldovan IuliaCMoldovan changed the title fix timing issues on fmcomms2 and ad9081 vcu118 Fix timing issues on FMCOMMS2 and AD9081/VCU118 Sep 10, 2025
@phlfm phlfm force-pushed the fix_timing_fmcomms2_and_ad9081 branch from ba373d6 to 2cc0018 Compare September 10, 2025 13:42
@phlfm
Copy link
Collaborator Author

phlfm commented Sep 10, 2025

Split into more atomic commits as requested by @acostina

@phlfm
Copy link
Collaborator Author

phlfm commented Sep 15, 2025

RetriggerCI

Copy link
Contributor

@LBFFilho LBFFilho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and everything builds fine, it would be nice to test it on hardware before merging.

@caosjr
Copy link
Contributor

caosjr commented Sep 19, 2025

system_bd.tcl scripts are not using your BOARD_NAME variable.

@bia1708
Copy link
Collaborator

bia1708 commented Sep 22, 2025

RetriggerCI

@phlfm phlfm requested a review from StancaPop as a code owner September 25, 2025 20:02
@phlfm
Copy link
Collaborator Author

phlfm commented Sep 26, 2025

RetriggerCI

IuliaCMoldovan
IuliaCMoldovan previously approved these changes Oct 10, 2025
@IuliaCMoldovan IuliaCMoldovan requested review from sarpadi and removed request for sarpadi October 10, 2025 08:52
PopPaul2021
PopPaul2021 previously approved these changes Oct 13, 2025
Copy link
Contributor

@PopPaul2021 PopPaul2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

sarpadi
sarpadi previously approved these changes Oct 13, 2025
@phlfm phlfm force-pushed the fix_timing_fmcomms2_and_ad9081 branch from 7fed80d to 070ead4 Compare October 13, 2025 19:32
@phlfm
Copy link
Collaborator Author

phlfm commented Oct 13, 2025

Agglutinated similar commits per project and logical group as suggested by @sarpadi (no changes made)

@phlfm phlfm force-pushed the fix_timing_fmcomms2_and_ad9081 branch 2 times, most recently from 0955d61 to ee11c3c Compare October 13, 2025 19:52
@AndreiGrozav
Copy link
Contributor

You placed auto_timing_fix.tcl in hdl/scripts
To me it seems that hdl/project/scripts/auto_timing_fix.tcl would be more appropriate, keeping a consistency with the type of scripts.

phlfm added 6 commits October 14, 2025 16:54
Signed-off-by: Pedro Mendonca <Pedro.Mendonca@analog.com>
- added new build flag (ADI_POST_ROUTE_SCRIPT)
- fixed typo
- removed redundant elsif
- updated build docs to build_hdl.rst

Signed-off-by: Pedro Mendonca <Pedro.Mendonca@analog.com>
- added auto timing fix (ATF) script as post route script
- changed strategy to Congestion_SpreadLogic_high
- refactored system_project.tcl as Factory Method
- updated header copyright year

Signed-off-by: Pedro Mendonca <Pedro.Mendonca@analog.com>
- added auto timing fix (ATF) script as post route script
- changed strategy to Congestion_SpreadLogic_high

Signed-off-by: Pedro Mendonca <Pedro.Mendonca@analog.com>
- added auto timing fix (ATF) script as post route script
- changed strategy to Congestion_SpreadLogic_high
- refactored system_project.tcl as Factory Method
- updated header copyright year

Signed-off-by: Pedro Mendonca <Pedro.Mendonca@analog.com>
- added auto timing fix (ATF) script as post route script
- changed strategy to Congestion_SpreadLogic_high
- refactored system_project.tcl as Factory Method
- updated header copyright year

Signed-off-by: Pedro Mendonca <Pedro.Mendonca@analog.com>
@phlfm phlfm dismissed stale reviews from IuliaCMoldovan, PopPaul2021, and sarpadi via 58b97d0 October 14, 2025 19:55
@phlfm phlfm force-pushed the fix_timing_fmcomms2_and_ad9081 branch from ee11c3c to 58b97d0 Compare October 14, 2025 19:55
@phlfm
Copy link
Collaborator Author

phlfm commented Oct 14, 2025

Moved ATF script to projects/scripts as suggested by @AndreiGrozav

@phlfm
Copy link
Collaborator Author

phlfm commented Oct 14, 2025

RetriggerCI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants